Conversation
This adds the external API query text to the extension directly to avoid users having to copy the query to their local `codeql` submodule or having to checkout a specific branch. This is a temporary solution until the queries are stabilized. Once they are, we will upstream these to `github/codeql` and use them like other contextual queries. Since this is just a temporary solution, this is not the prettiest code and is not intended to be a long-term solution. It does not do proper caching and will create a new temporary directory for every query run. The performance hit of this is acceptable and expected.
cf0a36c to
5200871
Compare
starcke
left a comment
There was a problem hiding this comment.
Generally looks good to me, just a question about whether we can simplify.
| @@ -0,0 +1,6 @@ | |||
| export type Query = { | |||
| mainQuery: string; | |||
| dependencies?: { | |||
There was a problem hiding this comment.
I wonder if we should just inline ExternalApi.qll, so that we just have a single query file that we run instead of having to store multiple files?
There was a problem hiding this comment.
I think it would be easier to transfer these queries to the codeql repository if we leave them as separate files. So far, we've not had to make any modifications to the ExternalApi.qll file, so we would only need to add in the main query and not make any changes to the ExternalApi.qll file. If we do combine these files, we'd need to split them out again in the future.
There was a problem hiding this comment.
I dont think it would be too hard to do that split up later, but I dont feel strongly. It also doesnt seem super complicated to have extra dependency files because we also need to have the synthetic pack. So happy to leave it like this.
| import { fetchExternalApisQuery as javaFetchExternalApisQuery } from "./java"; | ||
| import { Query } from "./query"; | ||
|
|
||
| export const fetchExternalApiQueries: Record<string, Query> = { |
There was a problem hiding this comment.
Just a question, probably fine. Are languages just string or do we have a type for them?
There was a problem hiding this comment.
We do, but the DatabaseItem.language is a string, so we'd need to cast it to a QueryLanguage which might hide problems rather than actually giving us type safety. I've changed it to use a QueryLanguage though since that will prevent us from adding unsupported types.
|
If we want to go with just a single file the following should work: |
This adds the external API query text to the extension directly to avoid users having to copy the query to their local
codeqlsubmodule or having to checkout a specific branch.This is a temporary solution until the queries are stabilized. Once they are, we will upstream these to
github/codeqland use them like other contextual queries.Since this is just a temporary solution, this is not the prettiest code and is not intended to be a long-term solution. It does not do proper caching and will create a new temporary directory for every query run. The performance hit of this is acceptable and expected.
Checklist
ready-for-doc-reviewlabel there.